Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate PoL into consensus #721

Merged
merged 29 commits into from
Sep 10, 2024
Merged

Integrate PoL into consensus #721

merged 29 commits into from
Sep 10, 2024

Conversation

zeegomo
Copy link
Contributor

@zeegomo zeegomo commented Sep 3, 2024

Unfortunately, testing now takes a lot longer since proving PoL on M2 takes ~10s, not sure about CI

@bacv
Copy link
Member

bacv commented Sep 3, 2024

Did you try macos-latest runner, they are supposed to be m1. m2 should be coming if not available yet.

@@ -29,6 +29,8 @@ jobs:
profile: minimal
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if its worth it, but we may have a separated workflow for risc0 if needed. Wdyt @bacv ?

Comment on lines +78 to +87
let res = tokio::task::spawn_blocking(move || {
Risc0LeaderProof::build(
public_inputs,
LeaderPrivate {
input,
input_cm_path,
},
)
})
.await;
Copy link
Collaborator

@danielSanchezQ danielSanchezQ Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a heavy operation? I don't get the spawn_blocking to immediately await.
is this the proving part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this is heavy and cpu intenstive (i.e. does not yield).spawn_blocking uses a dedicated thread for this to avoid blocking other tasks

@danielSanchezQ
Copy link
Collaborator

Unfortunately, testing now takes a lot longer since proving PoL on M2 takes ~10s, not sure about CI

Verifying is cheap right?

@zeegomo
Copy link
Contributor Author

zeegomo commented Sep 3, 2024

Verifying is cheap right?

yes, verifying takes ~13ms on my laptop

@zeegomo
Copy link
Contributor Author

zeegomo commented Sep 4, 2024

Apparently risc0 does not work on Windows. How would you be with dropping that for now?

@danielSanchezQ
Copy link
Collaborator

Apparently risc0 does not work on Windows. How would you be with dropping that for now?

It doesnt work here or risc0 itself does not support windows? The later would be a big problem imo

@zeegomo
Copy link
Contributor Author

zeegomo commented Sep 4, 2024

I'm afraid it's the latter but:

  • We plan to switch away from Risc0 as the base proofs (even beacause they are big), so you won't need it to validate incoming blocks
  • We can use docker in the meantime?

@danielSanchezQ
Copy link
Collaborator

I'm afraid it's the latter but:

* We plan to switch away from Risc0 as the base proofs (even beacause they are big), so you won't need it to validate incoming blocks

* We can use docker in the meantime?

There should be no problem in moving away from windows temporary. But whatever we choose should be available widely across OSs. I'll bubble up this in our channels.

Copy link
Collaborator

@danielSanchezQ danielSanchezQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A++ grade stuff. Thanks a lot!

@zeegomo zeegomo merged commit f2dfd46 into master Sep 10, 2024
8 checks passed
@zeegomo zeegomo deleted the add-cl-crate branch September 10, 2024 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants